Skip to content

phys/parameters: throw on missing output directory (issue #300 bug #1)#361

Merged
PDoakORNL merged 2 commits into
CompFUSE:masterfrom
tylersax:issue-300-1
Jun 1, 2026
Merged

phys/parameters: throw on missing output directory (issue #300 bug #1)#361
PDoakORNL merged 2 commits into
CompFUSE:masterfrom
tylersax:issue-300-1

Conversation

@tylersax

@tylersax tylersax commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

A non-existent output.directory previously surfaced only as a cryptic HDF5 crash. Add OutputParameters::validate(), called from Parameters::readInput on first rank only, which throws std::invalid_argument with a clear message naming the missing directory.

Validation is kept separate from OutputParameters::readWrite so the parsing path stays disk-free and unit-testable; this also avoids needing the existing ReadAll fixture to point at a real path. Follow-up PRs for bugs 2–4 are expected to add validate() to other parameter sections under the same convention.

Test plan

  • New InputValidationTest.MissingOutputDirectoryThrows exercises a JSON fixture pointing at a nonexistent path; expects std::invalid_argument.
  • All 16 existing parameter unit tests pass locally
  • CI build + test against origin/master post-rebase.

…bug CompFUSE#1)

A non-existent output.directory previously surfaced only as a cryptic HDF5
crash. Add OutputParameters::validate(), called from Parameters::readInput
on first rank only, which throws std::invalid_argument with a clear message
naming the missing directory.

Validation is kept separate from OutputParameters::readWrite so the parsing
path stays disk-free and unit-testable; this also avoids needing the existing
ReadAll fixture to point at a real path. Follow-up PRs for bugs CompFUSE#2CompFUSE#4 are
expected to add validate() to other parameter sections under the same
convention.
@tylersax tylersax marked this pull request as ready for review May 28, 2026 17:25

@PDoakORNL PDoakORNL left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the extra test file and directory this looks good to me, once that is fixed this looks to resolve #300 1.

Comment thread test/unit/phys/parameters/input_validation/input_validation_test.cpp Outdated
@tylersax

Copy link
Copy Markdown
Contributor Author

Great - thanks for the pointer. Moved both the test and the .json fixture to their appropriate home.

@PDoakORNL PDoakORNL self-requested a review June 1, 2026 13:57

@PDoakORNL PDoakORNL left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good now.

@tylersax

tylersax commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

CI is passing. Ready to merge when you are, @PDoakORNL

@PDoakORNL PDoakORNL merged commit eccecf6 into CompFUSE:master Jun 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants